Skip to content

feat(wallpaper): add folder-based wallpaper selection and shuffle - #2990

Open
greenbugx wants to merge 4 commits into
AvengeMedia:masterfrom
greenbugx:feat/wallpaper-folder
Open

feat(wallpaper): add folder-based wallpaper selection and shuffle#2990
greenbugx wants to merge 4 commits into
AvengeMedia:masterfrom
greenbugx:feat/wallpaper-folder

Conversation

@greenbugx

Copy link
Copy Markdown

Description

Adds a dedicated folder picker in the Settings wallpaper tab, allowing users to explicitly select an entire directory for automatic cycling without needing to manually select a dummy file.

Additionally, introduces a new Shuffle toggle to the Automatic Cycling configuration. When enabled, the background service seamlessly pipes the dynamically queried wallpaper list to shuf instead of sort, ensuring a truly random wallpaper is picked on every cycle while immediately adapting to any external file changes in the directory (e.g. from Nix or sync tools).

Note: Depends on the dank-qml-common FileBrowser folderMode fix submitted in this PR to resolve folder highlighting edge cases.

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that changes existing behavior)
  • Refactor / internal cleanup
  • Documentation
  • Other

Related issues

Closes #2833

Screenshots / video

image image
recording.mp4

Checklist

  • My code follows the conventions in CONTRIBUTING.md
  • I have tested my changes locally
  • New user-facing strings are wrapped in I18n.tr() with translator context, reusing existing terms where possible
  • Go changes: ran make fmt, added/updated tests, make test passes, and go mod tidy is clean
  • QML changes: ran make lint-qml with no new warnings
  • I have opened a corresponding pull request in dlx-docs to document any new behaviors: https://github.com/AvengeMedia/DankLinux-Docs

@greenbugx
greenbugx force-pushed the feat/wallpaper-folder branch from f8a6db0 to 83c600a Compare August 5, 2026 15:48

@bbedward bbedward left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I merged the qml-common change, so you can bump the submodule ref in this PR

Comment thread quickshell/Modules/Settings/WallpaperTab.qml Outdated
@greenbugx
greenbugx force-pushed the feat/wallpaper-folder branch from a767f9a to f298161 Compare August 7, 2026 16:56
@greenbugx
greenbugx requested a review from bbedward August 7, 2026 19:41
folderMode: true
showHiddenFiles: true
onFileSelected: path => {
var folderPath = path.startsWith("file://") ? path.substring(7) : path;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onFileSelected callback already strips the file:// from path, so this can be removed. We also prefer the Paths.strip helper in general where it is needed

onFileSelected: path => {
var folderPath = path.startsWith("file://") ? path.substring(7) : path;
var targetMonitor = SessionData.perMonitorWallpaper ? selectedMonitorName : "";
WallpaperCyclingService.cycleToNextWallpaper(targetMonitor, folderPath + "/dummy.jpg");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can add a proper function in WallpaperCyclingService, would make it clearer than using dummy.jpg

function cycleFromFolder(screenName, folderPath) {
    listWallpapers(folderPath, files => applyFirstWallpaper(screenName, files));
}

@greenbugx
greenbugx force-pushed the feat/wallpaper-folder branch from f298161 to 8837a22 Compare August 17, 2026 17:52
@greenbugx

Copy link
Copy Markdown
Author

@bbedward changes are done, please check again.

@greenbugx
greenbugx force-pushed the feat/wallpaper-folder branch from 8837a22 to 429b178 Compare August 19, 2026 17:58
@bbedward

Copy link
Copy Markdown
Collaborator

/claude review

Comment on lines +194 to +203
process.currentWallpaper = "";
process.goToPrevious = false;
process.running = true;
return;
}

var globalProcess = cyclingProcess;
globalProcess.command = findCommand(folderPath);
globalProcess.targetScreenName = screenName || "";
globalProcess.currentWallpaper = "";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passing currentWallpaper = "" makes applyCycledWallpaper skip the first image in the folder. With currentPath === "", findIndex returns -1, so currentIndex is coerced to 0 and the non-random branch computes targetIndex = (0 + 1) % length → the second file alphabetically. So selecting a folder with 2+ wallpapers never applies the first one (and with random cycling on, the do/while excludes index 0, so the first image can't be picked on the initial selection either). Only the single-file case works, because 1 % 1 === 0.

Either apply wallpaperList[0] directly for the folder-select path (the applyFirstWallpaper shape suggested earlier), or pass a sentinel that applyCycledWallpaper understands as "start at the first entry" instead of reusing the "next from current" logic.

if (!folderPath)
return;

SessionData.wallpaperCyclingFolderPath = folderPath;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wallpaperCyclingFolderPath is written here (and declared in SessionData.qml / SessionSpec.js) but never read anywhere — no other QML, and buildServerConfig() doesn't send it to the server either. So the "persist folder selection across restarts" goal isn't actually achieved; after a restart cycling still derives the directory from SessionData.wallpaperPath in cycle(). Either consume it (e.g. prefer it over the current wallpaper's dirname in cycle()) or drop the setting so it isn't persisted dead state.


FileBrowserModal {
parentModal: root.parentModal
browserTitle: I18n.tr("Select Wallpaper Folder", "wallpaper folder file browser title")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Select Wallpaper Folder is a new term, but Choose wallpaper folder already exists in quickshell/translations/en.json and is already translated in several locales. CONTRIBUTING asks to reuse existing terms where possible.

Suggested change
browserTitle: I18n.tr("Select Wallpaper Folder", "wallpaper folder file browser title")
browserTitle: I18n.tr("Choose wallpaper folder", "wallpaper folder file browser title")

@claude

claude Bot commented Aug 20, 2026

Copy link
Copy Markdown

Claude review

Feature works end-to-end, but folder selection skips the first wallpaper and the new persisted setting is never read.

  • Folder select applies the second image, not the first — currentWallpaper = "" makes applyCycledWallpaper fall through to (0+1) % length, quickshell/Services/WallpaperCyclingService.qml:194-203
  • wallpaperCyclingFolderPath is written but never read anywhere, so folder selection is not actually restored across restarts, quickshell/Services/WallpaperCyclingService.qml:187
  • New term Select Wallpaper Folder duplicates the already-translated Choose wallpaper folder, quickshell/Modules/Settings/WallpaperTab.qml:1377

Checked: cycling logic against the files.length < 1 change (single-file folders are fine), the folderMode FileBrowser contract in the pinned dank-qml-common, per-monitor routing, I18n contexts, and that en.json/template.json were not touched. Model: claude-opus-5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wallpaper folders

2 participants